matter/fast pair: Add support for merging hex files without partition manager #18792

nordicjm · 2024-11-11T06:58:42Z

For nRF54H20 only

Adds support for merging other files onto the uicr_merged.hex file Signed-off-by: Jamie McCrae <[email protected]>

Adds support for generating fast pair data for nRF54H20 which does not use partition manager Signed-off-by: Kamil Piszczek <[email protected]> Signed-off-by: Jamie McCrae <[email protected]>

NordicBuilder · 2024-11-11T06:59:13Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
matter	nrfconnect/sdk-connectedhomeip@`81eb1e9` (master)	nrfconnect/sdk-connectedhomeip#511	nrfconnect/sdk-connectedhomeip#511/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

NordicBuilder · 2024-11-11T07:01:13Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 4

Inputs:

Sources:

sdk-nrf: PR head: 2b3619b60e22623d9752b1445afd15b2b06472cf
matter: PR head: d7f660ac66f2018037af2dda6688918e8189b151

more details

sdk-nrf:

PR head: 2b3619b60e22623d9752b1445afd15b2b06472cf
merge base: f641783cba0fcf76ee906d702d1ae68ad3e1e364
target head (main): e4172b8cfa50fcd33401147603f7895854e9c135
Diff

matter:

PR head: d7f660ac66f2018037af2dda6688918e8189b151
merge base: 81eb1e99a9fd48bbf3bca2a56e7ac888c7f24a16
target head (master): 81eb1e99a9fd48bbf3bca2a56e7ac888c7f24a16
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (10)

cmake
│  ├── sysbuild
│  │  ├── fast_pair_hex.cmake
│  │  ├── suit.cmake
│  │  │ suit_utilities.cmake
modules
│  ├── lib
│  │  ├── matter
│  │  │  ├── config
│  │  │  │  ├── nrfconnect
│  │  │  │  │  ├── chip-module
│  │  │  │  │  │  │ generate_factory_data_sysbuild.cmake
samples
│  ├── bluetooth
│  │  ├── fast_pair
│  │  │  ├── input_device
│  │  │  │  ├── boards
│  │  │  │  │  │ nrf54h20dk_nrf54h20_cpuapp.overlay
│  │  │  │  │ sample.yaml
subsys
│  ├── bluetooth
│  │  ├── services
│  │  │  ├── fast_pair
│  │  │  │  ├── Kconfig.fast_pair
│  │  │  │  │ fp_registration_data.c
sysbuild
│  │ CMakeLists.txt
west.yml

Outputs:

Toolchain

Version: b81a7cd864
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:b81a7cd864_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister - Skipped: Skipping Build & Test as it succeeded in a previous run: 2
❌ Integration tests
- ✅ test-sdk-audio - Skipped: Job was skipped as it succeeded in a previous run
- ✅ desktop52_verification - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-boot - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test_ble_nrf_config - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-ble_mesh - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-ble_samples - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-chip - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-nfc - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-nrf-iot_cloud - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-nrf-iot_libmodem-nrf - Skipped: Job was skipped as it succeeded in a previous run
- ❌ test-fw-nrfconnect-nrf-iot_serial_lte_modem
- ✅ test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- ✅ test-fw-nrfconnect-nrf-iot_samples
- ✅ test-fw-nrfconnect-nrf-iot_lwm2m - Skipped: Job was skipped as it succeeded in a previous run
- ✅ doc-internal - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-nrf-iot_thingy91 - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-nrf_crypto - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-rpc - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-rs
- ✅ test-fw-nrfconnect-fem - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-tfm - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-thread - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-zigbee - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-sdk-find-my - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-nrf-iot_mosh - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-nrf-iot_positioning - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-sdk-sidewalk - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-sdk-wifi - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-low-level
- ✅ test-fw-nrfconnect-nrf-iot_nrf_provisioning - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-sdk-pmic-samples - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-sdk-mcuboot - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-sdk-dfu - Skipped: Job was skipped as it succeeded in a previous run
- ✅ test-fw-nrfconnect-ps - Skipped: Job was skipped as it succeeded in a previous run

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2024-11-11T07:15:08Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

NordicBuilder · 2024-11-11T08:11:29Z

Memory footprint analysis revealed the following potential issues

sample.matter.template.release[nrf7002dk/nrf5340/cpuapp]: High ROM usage: 811902[B] - link (cc: @kkasperczyk-no @ArekBalysNordic @markaj-nordic)

Note: This message is automatically posted and updated by the CI (latest/sdk-nrf/PR-18792/2)

Added nRF54H20 DK support to the Fast Pair Input Device sample. Ref: NCSDK-29602 Signed-off-by: Kamil Piszczek <[email protected]>

Updates matter to include support for generating factory data when partition manager is not used Signed-off-by: Jamie McCrae <[email protected]>

Calls the updated factory data function irrespective of partition manager enablement Signed-off-by: Jamie McCrae <[email protected]>

alstrzebonski

Looks good.

alstrzebonski · 2024-11-12T13:46:53Z

samples/bluetooth/fast_pair/input_device/boards/nrf54h20dk_nrf54h20_cpuapp.overlay

+		storage_partition: partition@1e3000 {
+			reg = <0x1e3000 DT_SIZE_K(20)>;
+		};
+
+		bt_fast_pair_partition: partition@1e8fb8 {
+			label = "bt_fast_pair";
+			reg = <0x1e8fb8 0x48>;
+		};


I'm not sure if the addresses are correct. 20kB is equal to 0x5000 so I think the bt_fast_pair_partition address should be set to 0x1e8000 (0x1e3000 + 0x5000) instead of 0x1e8fb8. Correct me if I'm wrong.

Well, I positioned it at the end of the 0x1000 memory chunk to mimick the PM solution in the case of automatic builds without the specific memory layout file (pm.yaml). On the other hand, we typically position the provisioning data at the beginning of the 0x1000 memory chunk in builds where we define the memory layout file (pm.yaml).

@alstrzebonski, I don't have a preference here. We can align it according to your suggestion or we can leave this part and think about the position of the fast pair provisioning data hex in the nRF54H20 memory in the follow-up task

I think it should be aligned to my suggestion or, if we want to mimic the PM, we should add the comment explaining the gap in the memory map. I'm okay with both solutions. It can be also left as is and improved in follow-up task - maybe that's the best option considering it's @nordicjm's PR. It will be easier to leave it as is for now.

@alstrzebonski, could you create a follow-up task?

https://nordicsemi.atlassian.net/browse/NCSDK-30269

kapi-no

The Fast Pair part looks good to me. The configurations aspects of Fast Pair (e.g. partition position) can be improved in the follow-up PRs

tejlmand

couple of observations and a small proposal improvement, but nothing blocking.

tejlmand · 2024-11-14T13:11:20Z

cmake/sysbuild/suit_utilities.cmake

+# dependencies that need to be generated before hand. This will overwrite existing data if it is
+# present in other files
+function(suit_add_merge_hex_file)
+  cmake_parse_arguments(VAR "" "" "FILES;DEPENDENCIES" ${ARGN})


better to start using arg as prefix.
See: https://gitlab.kitware.com/cmake/cmake/-/issues/25773, and https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9788/diffs

Suggested change

cmake_parse_arguments(VAR "" "" "FILES;DEPENDENCIES" ${ARGN})

cmake_parse_arguments(arg "" "" "FILES;DEPENDENCIES" ${ARGN})

tejlmand · 2024-11-14T13:15:03Z

cmake/sysbuild/suit_utilities.cmake

+  if(NOT VAR_FILES)
+    message(FATAL_ERROR "suit_add_merge_hex_file missing required argument FILES")
+  endif()


Can be simplified and also ensure common way of printing argument errors:

Suggested change

if(NOT VAR_FILES)

message(FATAL_ERROR "suit_add_merge_hex_file missing required argument FILES")

endif()

zephyr_check_arguments_required(${CMAKE_CURRENT_FUNCTION} arg FILES)

tejlmand · 2024-11-14T13:17:19Z

cmake/sysbuild/suit.cmake

    set_property(
      GLOBAL APPEND PROPERTY SUIT_POST_BUILD_COMMANDS
      COMMAND ${PYTHON_EXECUTABLE} ${ZEPHYR_BASE}/scripts/build/mergehex.py
      --overlap replace
      -o ${OUTPUT_HEX_FILE}
-       ${ARTIFACTS_TO_MERGE}
+       ${ARTIFACTS_TO_MERGE} ${merge_files}


observation: this is really not self-explaining what the difference between ARTIFACTS_TO_MERGE and merge_files is.
But as this file in general is messy, then there is not change requested for this PR.

nordicjm and others added 2 commits November 11, 2024 06:54

cmake: sysbuild: suit: Add support for merging other files

6c06685

Adds support for merging other files onto the uicr_merged.hex file Signed-off-by: Jamie McCrae <[email protected]>

cmake: sysbuild: fast_pair: add support for nRF54H20 non-PM

53b9449

Adds support for generating fast pair data for nRF54H20 which does not use partition manager Signed-off-by: Kamil Piszczek <[email protected]> Signed-off-by: Jamie McCrae <[email protected]>

nordicjm added this to the 2.9.0 milestone Nov 11, 2024

nordicjm requested review from a team as code owners November 11, 2024 06:58

github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Nov 11, 2024

NordicBuilder added manifest-matter DNM labels Nov 11, 2024

kapi-no and others added 3 commits November 11, 2024 10:02

samples: bluetooth: fast_pair: input_device: add support for nrf54h20dk

c51efb6

Added nRF54H20 DK support to the Fast Pair Input Device sample. Ref: NCSDK-29602 Signed-off-by: Kamil Piszczek <[email protected]>

manifest: Update sdk-connectedhomeip

7491f01

Updates matter to include support for generating factory data when partition manager is not used Signed-off-by: Jamie McCrae <[email protected]>

sysbuild: Add support for matter non-PM factory data generation

2b3619b

Calls the updated factory data function irrespective of partition manager enablement Signed-off-by: Jamie McCrae <[email protected]>

nordicjm force-pushed the fast_pair_dts_provisioning_wip branch from 4df741a to 2b3619b Compare November 11, 2024 10:02

alstrzebonski reviewed Nov 12, 2024

View reviewed changes

kapi-no approved these changes Nov 12, 2024

View reviewed changes

alstrzebonski self-requested a review November 12, 2024 14:37

alstrzebonski approved these changes Nov 12, 2024

View reviewed changes

maciejpietras requested review from robertstypa and ahasztag November 12, 2024 16:46

robertstypa approved these changes Nov 13, 2024

View reviewed changes

kkasperczyk-no approved these changes Nov 14, 2024

View reviewed changes

tejlmand approved these changes Nov 14, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

matter/fast pair: Add support for merging hex files without partition manager #18792

matter/fast pair: Add support for merging hex files without partition manager #18792

nordicjm commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024 •

edited

Loading

NordicBuilder commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024 •

edited

Loading

alstrzebonski left a comment

alstrzebonski Nov 12, 2024 •

edited

Loading

nordicjm Nov 12, 2024

kapi-no Nov 12, 2024

alstrzebonski Nov 12, 2024

kapi-no Nov 12, 2024

alstrzebonski Nov 13, 2024

kapi-no left a comment

tejlmand left a comment

tejlmand Nov 14, 2024

tejlmand Nov 14, 2024

tejlmand Nov 14, 2024

	cmake_parse_arguments(VAR "" "" "FILES;DEPENDENCIES" ${ARGN})
	cmake_parse_arguments(arg "" "" "FILES;DEPENDENCIES" ${ARGN})

matter/fast pair: Add support for merging hex files without partition manager #18792

Are you sure you want to change the base?

matter/fast pair: Add support for merging hex files without partition manager #18792

Conversation

nordicjm commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

NordicBuilder commented Nov 11, 2024

NordicBuilder commented Nov 11, 2024 • edited Loading

Memory footprint analysis revealed the following potential issues

alstrzebonski left a comment

Choose a reason for hiding this comment

alstrzebonski Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kapi-no left a comment

Choose a reason for hiding this comment

tejlmand left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NordicBuilder commented Nov 11, 2024 •

edited

Loading

NordicBuilder commented Nov 11, 2024 •

edited

Loading

alstrzebonski Nov 12, 2024 •

edited

Loading